skeleton POC #16728
skeleton POC #16728
Conversation
|
New Issues (17)Checkmarx found the following issues in this Pull Request
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16728 +/- ##
==========================================
- Coverage 38.81% 38.79% -0.02%
==========================================
Files 3406 3418 +12
Lines 96640 96963 +323
Branches 14510 14561 +51
==========================================
+ Hits 37509 37617 +108
- Misses 57493 57694 +201
- Partials 1638 1652 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
| /** | ||
| * The shape of the corners of the skeleton element | ||
| */ | ||
| readonly edgeShape = input<"box" | "circle">("box"); |
There was a problem hiding this comment.
Does it make more sense for this to just be 'shape'?
There was a problem hiding this comment.
Maybe! I felt like shape="circle" would imply it's always a circle versus just the corners, but I could be overthinking it
There was a problem hiding this comment.
Yeah, I guess the consumer could pass circle and then give it differing width and heights. Might be confusing. Unless if we added width and height inputs. Then, if `shape='circle' we only take one of those values... 🤔 Do we think consumers will want full control to set styles with tailwind? Or are the width/height inputs a less complex DX?
There was a problem hiding this comment.
I thought about doing width/height inputs but it felt kind of silly when it's super simple with tailwind already, like I'd just be re-implementing classes. The designs have non-circle shapes with rounded corners so that is why I didn't want to imply the object itself being a circle
There was a problem hiding this comment.
Ah, got ya. In that case, I suppose we can't assume everything will be circular.
re width/height inputs: I'm not sure the assumption that it's easy to do with tailwind is necessarily true for everyone. We find it easy because we use tailwind every day but, some folks may not be as familiar. Explicit inputs might feel easier for them. IDK which is 'better' per se though
| /** | ||
| * Array-transformed version of the `lines` to loop over | ||
| */ | ||
| protected linesArray = computed(() => Array.from(Array(this.lines()).keys())); |
There was a problem hiding this comment.
I think we could simplify this a little bit if we want
[...Array(this.lines()).keys()]
🎟️ Tracking
📔 Objective
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes